From c122e600cff461db7d77dee42e52d128f4060c39 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 2 Jun 2017 07:21:50 -0700 Subject: [PATCH] Cut down allocations in Display impls Avoid unnecessary `String` allocations in hot paths that get run a lot for large graphs. --- src/cargo/core/package_id.rs | 8 +-- src/cargo/core/resolver/encode.rs | 2 +- src/cargo/core/source.rs | 100 +++++++++++++++++------------- src/cargo/sources/git/source.rs | 2 +- 4 files changed, 62 insertions(+), 50 deletions(-) diff --git a/src/cargo/core/package_id.rs b/src/cargo/core/package_id.rs index 1e906d97d..4058eeaac 100644 --- a/src/cargo/core/package_id.rs +++ b/src/cargo/core/package_id.rs @@ -28,10 +28,10 @@ impl ser::Serialize for PackageId { fn serialize(&self, s: S) -> Result where S: ser::Serializer { - let source = self.inner.source_id.to_url(); - let encoded = format!("{} {} ({})", self.inner.name, self.inner.version, - source); - encoded.serialize(s) + s.collect_str(&format_args!("{} {} ({})", + self.inner.name, + self.inner.version, + self.inner.source_id.to_url())) } } diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs index 403c01e51..3a93c2047 100644 --- a/src/cargo/core/resolver/encode.rs +++ b/src/cargo/core/resolver/encode.rs @@ -267,7 +267,7 @@ impl ser::Serialize for EncodablePackageId { fn serialize(&self, s: S) -> Result where S: ser::Serializer, { - self.to_string().serialize(s) + s.collect_str(self) } } diff --git a/src/cargo/core/source.rs b/src/cargo/core/source.rs index 9f32d692d..e7a3ce4a2 100644 --- a/src/cargo/core/source.rs +++ b/src/cargo/core/source.rs @@ -173,34 +173,8 @@ impl SourceId { } } - pub fn to_url(&self) -> String { - match *self.inner { - SourceIdInner { kind: Kind::Path, ref url, .. } => { - format!("path+{}", url) - } - SourceIdInner { - kind: Kind::Git(ref reference), ref url, ref precise, .. - } => { - let ref_str = reference.url_ref(); - - let precise_str = if precise.is_some() { - format!("#{}", precise.as_ref().unwrap()) - } else { - "".to_string() - }; - - format!("git+{}{}{}", url, ref_str, precise_str) - } - SourceIdInner { kind: Kind::Registry, ref url, .. } => { - format!("registry+{}", url) - } - SourceIdInner { kind: Kind::LocalRegistry, ref url, .. } => { - format!("local-registry+{}", url) - } - SourceIdInner { kind: Kind::Directory, ref url, .. } => { - format!("directory+{}", url) - } - } + pub fn to_url(&self) -> SourceIdToUrl { + SourceIdToUrl { inner: &*self.inner } } // Pass absolute path @@ -350,7 +324,7 @@ impl ser::Serialize for SourceId { if self.is_path() { None::.serialize(s) } else { - Some(self.to_url()).serialize(s) + s.collect_str(&self.to_url()) } } } @@ -372,7 +346,10 @@ impl fmt::Display for SourceId { } SourceIdInner { kind: Kind::Git(ref reference), ref url, ref precise, .. } => { - write!(f, "{}{}", url, reference.url_ref())?; + write!(f, "{}", url)?; + if let Some(pretty) = reference.pretty_ref() { + write!(f, "?{}", pretty)?; + } if let Some(ref s) = *precise { let len = cmp::min(s.len(), 8); @@ -452,25 +429,60 @@ impl hash::Hash for SourceId { } } -impl GitReference { - pub fn to_ref_string(&self) -> Option { - match *self { - GitReference::Branch(ref s) => { - if *s == "master" { - None - } else { - Some(format!("branch={}", s)) +pub struct SourceIdToUrl<'a> { + inner: &'a SourceIdInner, +} + +impl<'a> fmt::Display for SourceIdToUrl<'a> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self.inner { + SourceIdInner { kind: Kind::Path, ref url, .. } => { + write!(f, "path+{}", url) + } + SourceIdInner { + kind: Kind::Git(ref reference), ref url, ref precise, .. + } => { + write!(f, "git+{}", url)?; + if let Some(pretty) = reference.pretty_ref() { + write!(f, "?{}", pretty)?; + } + if let Some(precise) = precise.as_ref() { + write!(f, "#{}", precise)?; } + Ok(()) + } + SourceIdInner { kind: Kind::Registry, ref url, .. } => { + write!(f, "registry+{}", url) + } + SourceIdInner { kind: Kind::LocalRegistry, ref url, .. } => { + write!(f, "local-registry+{}", url) + } + SourceIdInner { kind: Kind::Directory, ref url, .. } => { + write!(f, "directory+{}", url) } - GitReference::Tag(ref s) => Some(format!("tag={}", s)), - GitReference::Rev(ref s) => Some(format!("rev={}", s)), } } +} - fn url_ref(&self) -> String { - match self.to_ref_string() { - None => "".to_string(), - Some(s) => format!("?{}", s), +impl GitReference { + pub fn pretty_ref(&self) -> Option { + match *self { + GitReference::Branch(ref s) if *s == "master" => None, + _ => Some(PrettyRef { inner: self }), + } + } +} + +pub struct PrettyRef<'a> { + inner: &'a GitReference, +} + +impl<'a> fmt::Display for PrettyRef<'a> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self.inner { + GitReference::Branch(ref b) => write!(f, "branch={}", b), + GitReference::Tag(ref s) => write!(f, "tag={}", s), + GitReference::Rev(ref s) => write!(f, "rev={}", s), } } } diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 4a3f8f6f6..75c0281a4 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -107,7 +107,7 @@ impl<'cfg> Debug for GitSource<'cfg> { fn fmt(&self, f: &mut Formatter) -> fmt::Result { write!(f, "git repo at {}", self.remote.url())?; - match self.reference.to_ref_string() { + match self.reference.pretty_ref() { Some(s) => write!(f, " ({})", s), None => Ok(()) } -- 2.30.2